Skip to content

feat(sqlserver): Part 8 — dbt-sqlserver Jinja macro package - #18

Merged
axellpadilla merged 2 commits into
sqlserver-v2-portfrom
part-8-sqlserver-macros
Aug 3, 2026
Merged

feat(sqlserver): Part 8 — dbt-sqlserver Jinja macro package#18
axellpadilla merged 2 commits into
sqlserver-v2-portfrom
part-8-sqlserver-macros

Conversation

@axellpadilla

Copy link
Copy Markdown
Collaborator

Stacked on #17 (Part 7).

Vendors v1's 34-file macro tree into crates/dbt-loader/src/dbt_macro_assets/dbt-sqlserver, mirroring dbt-fabric's directory layout rather than v1's. Non-mechanical decisions, each checked against v1 source, the shared dbt-adapters package, or adapter_impl.rs:

  • No sqlserver__generate_schema_name override — falls through to the shared default__generate_schema_name (standard concatenation), since Part 7 registered no SqlServer behavior flags and v2 has no per-adapter flag mechanism to gate it. This is what Flip dbt_sqlserver_use_default_schema_concat's default to True in the final 1.12 release dbt-msft/dbt-sqlserver#800 already assumes.
  • hooks.sql's two branches on adapter.behavior.dbt_sqlserver_use_dbt_transactions emit identical SQL either way, so the conditional (which would reference a nonexistent behavior flag in v2) was dropped rather than kept.
  • full_refresh_build=prebuilt, table_refresh_method=dml, and a configured indexes: all raise a compiler error naming the gap, rather than silently running a different path or no-op'ing — matching the loud-failure precedent from Part 7's plan for deferred features.
  • Dropped everything gated on adapter.resolve_masks/apply_masks and index reconciliation on persisted tables — neither has a v2 Rust counterpart, and indexes: is already rejected loudly on first create so a persisted table can't have a nonempty config left to reconcile.
  • Skipped unit_test_create_table_as.sql (v2's tests/unit.sql dispatches through the same get_create_table_as_sql as every other build path — the v1 macro name is unreachable) and materializations/models/view/create_view_as.sql's sqlserver__create_view_exec (no caller anywhere in v1's own tree — dead code).
  • Ported adapters/catalog.sql: dbt-fabric's equivalent is still live in v2 for dbt docs generate, and v1's version is kept over Fabric's since it fills table_comment/column_comment, consistent with porting the full persist_docs.sql (which Fabric punts on entirely).
  • Added build_model_constraints(target_relation) to table.sql and incremental's full-refresh branch — v1 doesn't call it (older framework), Fabric's newer materializations do; v2 needs the explicit call site for FK/PK contract enforcement to work.

Every rewritten (not verbatim-copied) file was diffed against its v1 source to confirm only the intended removal/addition shows up — no accidental changes to retained logic.

Verified

  • cargo check -p dbt-loader: clean
  • cargo test -p dbt-loader --test main: 47 passed (43 pre-existing + 4 new), 0 failed
  • New tests (tests/materializations/table.rs) follow the existing view.rs/incremental.rs pattern — render the materialization through MacroTestHarness, mock adapter calls, assert on what ran — covering the two things this PR actually changed: table.sql's rename-swap surviving the prebuilt/dml branch removal, and the indexes: config now raising instead of silently no-op'ing.

Closes #8

Vendors v1's macro tree (34 files) into crates/dbt-loader/src/dbt_macro_assets/dbt-sqlserver,
mirroring dbt-fabric's directory layout rather than v1's, per the porting guide's
delegate-to-fabric-where-behavior-matches instruction.

Non-mechanical decisions, each checked against the v1 source, the shared
dbt-adapters package, or the Rust adapter_impl.rs added in Part 7:

- No sqlserver__generate_schema_name override: v2 has no per-adapter behavior-flag
  mechanism (Part 7 registered none for SqlServer), so the macro falls through to
  the shared default__generate_schema_name (standard target.schema_custom
  concatenation) rather than hardcoding v1's current no-prefix default. This is
  what dbt-msft/dbt-sqlserver#800 (filed last session) already assumes.
- adapter_specific_behavior_flags() being empty also means
  adapter.behavior.dbt_sqlserver_use_dbt_transactions doesn't exist in v2.
  hooks.sql's two branches on that flag emit identical SQL either way, so the
  conditional was dropped rather than left referencing an undefined attribute.
- full_refresh_build=prebuilt and table_refresh_method=dml (table.sql,
  incremental.sql) raise a compiler error instead of running: both depend on
  macros this PR doesn't port (create_table_as_prebuilt, table_dml_refresh),
  matching the loud-failure precedent set for custom indexes in Part 7's plan.
- sqlserver__get_create_index_sql raises a compiler error naming the config
  option, rather than inheriting the shared default's silent no-op — a model
  with indexes: configured would otherwise build successfully with none.
- Dropped everything gated on adapter.resolve_masks/apply_masks (table.sql,
  incremental.sql, snapshot.sql) and reconcile_indexes on persisted tables:
  neither has a v2 Rust counterpart, and since indexes: is already rejected
  loudly on first create, an already-existing table can't have a nonempty
  config to reconcile.
- Skipped unit_test_create_table_as.sql and its check_for_nested_cte helper:
  v2's tests/unit.sql materialization dispatches through the same
  get_create_table_as_sql used by every other build path, not a separate
  unit_test_create_table_as macro — the v1 file is unreachable in v2's call graph.
- Skipped materializations/models/view/create_view_as.sql
  (sqlserver__create_view_exec): grepped v1's own tree and found no caller;
  dead code inherited from an earlier Fabric-derived layout.
- Ported adapters/catalog.sql, matching dbt-fabric/macros/adapters/catalog.sql
  still being live in v2 for `dbt docs generate` — the Part 5 Rust metadata
  module doesn't replace it. v1's version is kept over Fabric's: it fills
  table_comment/column_comment from sys.extended_properties, consistent with
  porting the full persist_docs.sql (which Fabric punts on entirely).
- build_model_constraints(target_relation) added to table.sql and the
  full-refresh branch of incremental.sql (v1 doesn't call it, Fabric's newer
  materializations do) — v1's Python framework invoked model-constraint DDL
  outside the adapter macro; v2 needs the explicit call site.

Verified: cargo check -p dbt-loader clean; cargo test -p dbt-loader --test main:
44 passed, 0 failed (pre-existing suite, unaffected by this change — no test
coverage added here, matching Parts 1-7's precedent of leaving test authorship
to a dedicated pass).

Closes #8
…failure

Follows the view.rs/incremental.rs pattern (render the materialization
through MacroTestHarness, mock adapter calls, assert on what ran) rather
than a bare parse-only check — matching how postgres/databricks/spark are
covered, and consistent with this suite's existing adapters (only 5 of ~17
have view.rs coverage, 2 have incremental.rs; sqlserver had none).

Targets the two behaviors this PR actually changed relative to v1: the
rename-into-target / existing-renamed-to-backup swap that survives after
trimming the full_refresh_build=prebuilt and table_refresh_method=dml
branches, and sqlserver__get_create_index_sql raising instead of the shared
default's silent no-op when `indexes:` is configured.

Found along the way: MacroTestHarness's default_mock_config()'s generic
`config.get(key, default=...)` fallback only unwraps a *positional* second
arg; dbt-adapters' own create_indexes calls it with a keyword `default=[]`,
which the fallback doesn't see, so config.get('indexes', default=[]) doesn't
return the intended empty list. Not something to fix in shared harness code
for this PR — config_mock() here hardcodes indexes/contract explicitly and
only falls through to args[1] for genuinely positional call sites.
@axellpadilla
axellpadilla changed the base branch from part-7-sqlserver-adapter-impl to sqlserver-v2-port August 3, 2026 01:18
@axellpadilla
axellpadilla merged commit 52b8adb into sqlserver-v2-port Aug 3, 2026
@axellpadilla axellpadilla linked an issue Aug 3, 2026 that may be closed by this pull request
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(sqlserver): Part 8 — dbt-sqlserver Jinja macro package

1 participant